Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Refactor] Standardize "damage taken" modifier usage #7130

Merged
merged 2 commits into from
Feb 25, 2025

Conversation

Xaver-DaRed
Copy link
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

This is current usage:

  • Physical SDT modifiers: Defined as a float in database, fetched as an uint, stored in a base 1000 modifier where base 100% damage = value of 1000
  • Elemental SDT modifiers: Defined as an int in database, fetched as an int, stored in a base 10000 modifier where base 100% damage = value of 0 (sane) BUT 50% less damage = 5000 and 50% more damage = -5000
  • Other SDT modifiers: Defined as an int in database, fetched as an int, stored in a base 10000 modifier where base 100% damage = value of 0, 50% less damage = -5000 and 50% more damage = 5000

This is new usage:

  • All SDT modifiers are defined as an int in database, fetched as an int, stored in a base 10000 modifier where base 100% damage = value of 0, 50% less damage = -5000 and 50% more damage = 5000

Steps to test these changes

Fight mobs. Cast spells.

@Xaver-DaRed Xaver-DaRed force-pushed the sdt-normalization branch 2 times, most recently from 9907d21 to 4e6c064 Compare February 23, 2025 22:49
@UmeboshiXI
Copy link
Contributor

Just thought of some not functional but for documentation. The mod enum has comments relating to the old physical SDTs not being the same base values as the others.

@zach2good
Copy link
Contributor

I don't know if this ever came up in the past, but should we rename these mods to imply that they're 1/10000 1e4-based, etc.

SLASH_SDT -> SLASH_SDT_BASE_10K? Just really hammer it home forever? I wouldn't be against us marking other things with types too: ..._FLOAT, _PERCENT, _something that indicates 0.0 to 1.0, etc.

@Xaver-DaRed
Copy link
Contributor Author

I wouldnt be opposed to changing the modifier naming conventions. Or implementing a convention in the first place.

With this change, now all _SDT mods use 10k as base and all function the same (positive -> more damage, negative .> less damage)
This was more something that was bound to happen. We started this a year or 2 ago and we just didnt finish it.

@zach2good
Copy link
Contributor

I think because this has been an issue for so long, and it's just a "thing you have to know" once you're up in Lua - and nobody is going to go read the comments next to where these mods/enums are defined: we should add things like _BASE_10K, _PERCENT, etc. to things that are of an arbitrary scale. We don't need to add this sort of annotation for things that line up in-game like DEF, etc.

@zach2good
Copy link
Contributor

We talked about this offline - the naming convention changes can happen in a later PR 👍

@zach2good zach2good merged commit 81bb331 into LandSandBoat:base Feb 25, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants